dashboard: review pane has no exit at desk width - #102
Conversation
Review is a mode, not a fourth column: opening it stands every other pane
down at every width. Below 760 px the tab bar takes you back, but above it
the tab bar is hidden and a tab click was the only thing that ever called
`show()` with another pane name — so an operator who clicked `N candidates
— review` on a desk could leave only by resizing the window or reloading
the page.
`back to map` in the review pane head, and Escape, call `panes.show('map')`.
Both are held off while a zone edit is live, as the floor picker and the
revision list already are: an edit has no autosave, and leaving would strand
it on a canvas nobody can see. A promotion returns to the map itself, the
decision the pane exists for having been made — unless the announcement
failed, where the note saying so is readable only in the pane that wrote it.
ui_test.mjs holds the exit in place: the pane has the control, its handler
names a pane that is not this one, Escape shares that handler, and an edit
disables it. browser_check.mjs drives it at 1600 px, where the tab bar
computes to `display: none`: the button returns to the map pane and the
operations panes are displayed again, re-entering works, Escape does the
same, and neither works mid-edit. 39/39 checks pass; the phone pass is
unchanged.
What the code and the check names already said did not need saying beside
them. What is left is what neither says: why 760 px decides the button
exists, why a promotion stays put when the announcement failed, the mutation
the `show('map')` assertion guards against, and why the browser check
re-enters the pane mid-flow.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // A promotion happened in the review pane: this pane's basemap is now a | ||
| // different map, so re-resolve it rather than keep drawing the old one. | ||
| function onPromoted() { | ||
| // different map, so re-resolve it rather than keep drawing the old one. The | ||
| // review is then over — except when the announcement failed, whose note is | ||
| // readable only in the pane that wrote it. | ||
| function onPromoted(site, floor, revision, announced) { | ||
| state.mapKey = null; | ||
| scheduleRender(); | ||
| if (announced) panes.show('map'); | ||
| } |
There was a problem hiding this comment.
Bug: a successful promotion navigates away before its own confirmation is visible, and lands on the wrong floor.
In promote()'s success branch, the outcome note ("<site>/<floor> is on <rev>") is written into #review-note and then this.onPromoted(site, floor, revision, Boolean(body.announced)) is called:
Mote/mote_fleet/server/ui/review.mjs
Lines 582 to 588 in 078bbe7
onPromoted here ignores its own site/floor args and, when announced is true, immediately calls panes.show('map'), which hides the review pane (.review-pane { display: none } in style.css) in the same tick — so the success note is written into a pane that's already hidden, and gets cleared (this.note('')) the next time review is opened. It's never seen.
Worse, the map pane doesn't re-resolve to the promoted floor: ensureMap() derives the basemap from the selected robot's pose/health site+floor, unrelated to what was just promoted via the review pane's own floor picker. In exactly the scenario this pane exists for — promoting a candidate for a floor no robot is currently reporting — the operator is dropped onto an unrelated floor (or "no floor reported"), with no visible evidence the promotion succeeded or which revision landed.
This is a regression from this PR: before it, onPromoted() only did state.mapKey = null; scheduleRender(); and the pane stayed open, so the note was readable.
Suggested direction: only auto-navigate when the promoted floor matches state.mapKey, or drive the map pane to the promoted site/floor instead of discarding those arguments, or surface the outcome on something that survives the pane switch.
1 participant
Review is a mode, not a fourth column: opening it stands every other pane down at every width. Below 760 px the tab bar takes you back, but above it the tab bar is
display: noneand a tab click was the only thing that ever calledpanes.show()with another pane name — so an operator who clickedN candidates — reviewon a desk could escape only by resizing the window or reloading the page. Introduced with the review view (#100), inherited by the zone editor.What changed
back to mapin the review pane head (index.html), wired inapp.mjstopanes.show('map'). Escape shares that handler, ondocumentbecause the pane holds no focus of its own.ReviewView.leavable()is the predicate,renderEditControlsdisables the button, andstyle.cssgives it the same half-opacity the other locked controls have. An edit has no autosave;cancelis how it ends.onPromotedtherefore takes the announced flag.docs/fleet/README.md§11 gains a paragraph on leaving the pane.Verification
ui_test.mjs(62 pass): the review pane contains an exit control; its handler names a pane that is not this one (ashow('review')on a button labelledbackwould look right in the markup and change nothing); Escape routes through the same handler; an edit disables it. Mutation-checked — pointingonReviewBackback atreviewfails the test.pixi run fleet-ui-check(39/39, up from 37): at 1600 px, with the tab bar measured asdisplay: none, the button returns to the map pane, the operations panes are displayed again (the:hasmode rule has let go), re-entering works, and Escape does the same —{"width":1600,"tabs":"none","button":"map","shown":"flex","reopened":"review","escape":"map"}. Mid-edit both are refused:{"disabled":true,"pane":"review"}. A promotion lands on the map. Mutation-checked the same way: 36/37 with the fix reverted.